feat(kilocode): configure and launch from cli#209
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR reworks orchestration into a privacy-masked Kilo webview flow, adds local AI provider management, shifts orchestration copy from preview-oriented to run-oriented language, and extends OpenAI chat provider selection and fallback handling with matching tests. ChangesOrchestration UI, privacy, and local AI
OpenAI chat provider management
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Implemented the privacy hotfix on top of current What changed:
Validation actually run:
@coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web-ui/modules/app.methods.task-orchestration.mjs (1)
259-269: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute the new redaction placeholders through i18n.
These helpers now surface hardcoded English strings, so non-English locales will still see
(output hidden)and(N log entries hidden)in the orchestration detail view even though this PR adds locale-backed privacy copy elsewhere.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.task-orchestration.mjs` around lines 259 - 269, The new redaction placeholders in formatTaskNodeLogs and formatTaskNodeOutputText are hardcoded English strings, so replace them with locale-backed messages from i18n. Update these helpers to use the existing translation mechanism for the hidden output/log variants, keeping the same conditional logic while routing both “no logs” and hidden-output text through the orchestration detail view’s localized privacy copy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-ui/modules/i18n/locales/ja.mjs`:
- Around line 972-977: The new orchestration privacy label for provider-hidden
is still in English, so update the Japanese locale entry in ja.mjs to use a
proper Japanese translation for the orchestration.privacy.providerHidden key.
Keep the surrounding privacy strings consistent with the other entries in this
section and verify the key remains aligned with the existing localization
object.
---
Outside diff comments:
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 259-269: The new redaction placeholders in formatTaskNodeLogs and
formatTaskNodeOutputText are hardcoded English strings, so replace them with
locale-backed messages from i18n. Update these helpers to use the existing
translation mechanism for the hidden output/log variants, keeping the same
conditional logic while routing both “no logs” and hidden-output text through
the orchestration detail view’s localized privacy copy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 52abbd79-44c2-493b-8c4e-3179642eec9e
📒 Files selected for processing (11)
tests/unit/config-tabs-ui.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (10)
tests/unit/config-tabs-ui.test.mjs (1)
159-169: LGTM!tests/unit/web-ui-logic.test.mjs (1)
1896-1896: LGTM!Also applies to: 2065-2067
web-ui/partials/index/panel-orchestration.html (7)
178-182: LGTM!Also applies to: 195-195
259-261: LGTM!Also applies to: 322-325, 348-351
397-407: LGTM!
409-441: LGTM!
482-487: 🔒 Security & PrivacyConfirm intended UX of switching
workspacePath/threadIdtotype="password".These are not secrets but user-editable values;
type="password"masks the user's own input, preventing them from verifying or editing the path/id they typed, and some browsers/extensions still treat password fields specially even withautocomplete="off". If the goal is only to avoid shoulder-surfing/screen-share leakage, a reveal toggle or a CSS text-masking approach would preserve editability while keeping the masking. Please confirm this tradeoff is intended.
548-548: LGTM!
408-408: 🔒 Security & PrivacyKeep redaction on the remaining orchestration errors.
web-ui/partials/index/panel-orchestration.html:145and:376still rendertaskOrchestration.lastErrorandtaskOrchestration.selectedRunErrorverbatim, and both are populated fromres.error/ caught exception messages. If either API can surface backend/provider detail, these paths need the same masking asrun.error.web-ui/res/web-ui-render.precompiled.js (1)
4258-4258: 🗄️ Data Integrity & IntegrationBundle matches the template for these privacy bindings.
|
Updated after the workspace selector/privacy re-check. Additional fixes:
Validation actually run after this update:
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Handled the remaining CodeRabbit locale comment. Additional fix:
Validation actually run after this update:
@coderabbitai review |
|
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! ✅ Action performedReview finished.
|
|
Updated this PR with
Validation actually run:
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Pushed
Validation actually run after this update:
Note: the test command still prints an existing @coderabbitai review |
|
✅ Action performedReview finished.
|
|
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web-ui/partials/index/panel-orchestration.html (2)
211-222: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNode IDs are still exposed in the “redacted” plan/detail views.
These blocks still render
wave.nodeIds,node.id, andnode.workflowId, so the UI can leak the exact orchestration identifiers this privacy hotfix is trying to hide. Please switch those fields to the same generic/count-based placeholders used elsewhere in this panel.Also applies to: 436-440
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/partials/index/panel-orchestration.html` around lines 211 - 222, The redacted orchestration detail views are still leaking exact identifiers in the wave/node render blocks. Update the template sections that iterate over taskOrchestration.plan.waves and taskOrchestration.plan.nodes so they no longer display wave.nodeIds, node.id, or node.workflowId. Replace those fields with the same generic/count-based placeholders already used elsewhere in this panel, keeping the visible structure but removing identifier-level detail.
54-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDrop the
menuroles here —<details>plus buttons already provides usable native interaction, butrole="menu"/role="menuitemradio"opt into the ARIA menu pattern without the arrow-key and focus management that pattern expects. Remove the roles or wire the full menu behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/partials/index/panel-orchestration.html` around lines 54 - 63, The task workspace selector in panel-orchestration.html is using ARIA menu semantics without the required keyboard/focus behavior. Update the markup around the task-workspace-menu-panel and the v-for button items to remove role="menu" and role="menuitemradio", or implement the full menu interaction pattern if you keep them. Keep the native details/button behavior centered around selectTaskWorkspace and the workspace.active state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web-ui/partials/index/panel-orchestration.html`:
- Around line 211-222: The redacted orchestration detail views are still leaking
exact identifiers in the wave/node render blocks. Update the template sections
that iterate over taskOrchestration.plan.waves and taskOrchestration.plan.nodes
so they no longer display wave.nodeIds, node.id, or node.workflowId. Replace
those fields with the same generic/count-based placeholders already used
elsewhere in this panel, keeping the visible structure but removing
identifier-level detail.
- Around line 54-63: The task workspace selector in panel-orchestration.html is
using ARIA menu semantics without the required keyboard/focus behavior. Update
the markup around the task-workspace-menu-panel and the v-for button items to
remove role="menu" and role="menuitemradio", or implement the full menu
interaction pattern if you keep them. Keep the native details/button behavior
centered around selectTaskWorkspace and the workspace.active state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7eaa0132-91e2-4982-a6d7-bbe0d4c0f02e
📒 Files selected for processing (4)
tests/unit/config-tabs-ui.test.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/task-orchestration.css
🚧 Files skipped from review as they are similar to previous changes (3)
- web-ui/styles/task-orchestration.css
- tests/unit/config-tabs-ui.test.mjs
- web-ui/res/web-ui-render.precompiled.js
|
Pushed Additional fixes:
Validation actually run after this update:
Note: the test command still prints the existing @coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web-ui/modules/app.methods.task-orchestration.mjs (2)
472-505: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winMask backend error text in orchestration toasts as well.
The success path is now generic, but the failure branches in these same flows still call
showMessage(res.error | message, 'error')on Line 484, Lines 499-500, Lines 517-518, Lines 532-534, Lines 769-770, and Lines 784-785. That still exposes raw server error text in the visible UI, which can leak the thread/workspace/run details this privacy hotfix is trying to hide. Use localized generic error toasts here and keep the raw message only in internal state/logging if needed.Also applies to: 507-540, 759-790
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.task-orchestration.mjs` around lines 472 - 505, The orchestration failure toasts still surface raw backend error text, so update the error branches in runTaskOrchestration and the other affected task-orchestration flows to use generic localized messages instead of res.error or error.message. Keep the detailed backend error only in internal return values, state, or logging if needed, and make sure showMessage is called with a sanitized message in the identified orchestration methods.
496-496: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRoute the new success toasts through
t(...).These newly changed strings are hardcoded Chinese, so English/Vietnamese sessions will now mix locales even though this PR adds orchestration i18n coverage elsewhere. Please switch them to translation keys instead of literals.
Also applies to: 528-528, 754-754, 781-781
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/app.methods.task-orchestration.mjs` at line 496, The success toast strings in task orchestration are still hardcoded Chinese, so route them through the i18n helper instead of passing literals to showMessage. Update the affected success paths in the task-orchestration methods (including the ones around the start, progress, completion, and related orchestration flows) to use t(...) translation keys so locale-specific sessions stay consistent.web-ui/res/web-ui-render.precompiled.js (1)
4074-4090: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a valid selection role for the workspace items.
aria-checkedon a plainbuttonis invalid ARIA, so the active workspace state won’t be announced correctly. Restore matching radio/menuitemradio semantics or switch to native radio controls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/res/web-ui-render.precompiled.js` around lines 4074 - 4090, The workspace item button in the workspace menu is using invalid ARIA by setting aria-checked on a plain button, so update the selection control in the workspace menu render logic to use matching radio-style semantics instead. Locate the template block that renders each workspace item and either change the element/role to menuitemradio or radio with the appropriate checked state, or replace it with native radio controls, while keeping the active workspace state announced correctly through the existing workspace.active handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 472-505: The orchestration failure toasts still surface raw
backend error text, so update the error branches in runTaskOrchestration and the
other affected task-orchestration flows to use generic localized messages
instead of res.error or error.message. Keep the detailed backend error only in
internal return values, state, or logging if needed, and make sure showMessage
is called with a sanitized message in the identified orchestration methods.
- Line 496: The success toast strings in task orchestration are still hardcoded
Chinese, so route them through the i18n helper instead of passing literals to
showMessage. Update the affected success paths in the task-orchestration methods
(including the ones around the start, progress, completion, and related
orchestration flows) to use t(...) translation keys so locale-specific sessions
stay consistent.
In `@web-ui/res/web-ui-render.precompiled.js`:
- Around line 4074-4090: The workspace item button in the workspace menu is
using invalid ARIA by setting aria-checked on a plain button, so update the
selection control in the workspace menu render logic to use matching radio-style
semantics instead. Locate the template block that renders each workspace item
and either change the element/role to menuitemradio or radio with the
appropriate checked state, or replace it with native radio controls, while
keeping the active workspace state announced correctly through the existing
workspace.active handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aa08203e-dff7-4e11-bf15-6bceb42be9b4
📒 Files selected for processing (10)
tests/unit/config-tabs-ui.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.js
✅ Files skipped from review due to trivial changes (2)
- web-ui/modules/i18n/locales/ja.mjs
- web-ui/modules/i18n/locales/zh-tw.mjs
🚧 Files skipped from review as they are similar to previous changes (4)
- web-ui/modules/i18n/locales/zh.mjs
- tests/unit/web-ui-logic.test.mjs
- tests/unit/config-tabs-ui.test.mjs
- web-ui/partials/index/panel-orchestration.html
|
Update pushed in What changed:
Validation actually run:
Note: the existing @coderabbitai re-review |
|
✅ Action performedReview finished.
|
|
Implemented the Kilo-compatible orchestration host on this existing PR branch. What changed:
Validation performed:
Known existing noise:
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
web-ui/modules/i18n/locales/zh.mjs (1)
1242-1259: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd the missing empty-state strings for node output and logs.
formatTaskNodeOutputText()andformatTaskNodeLogs()now look uporchestration.detail.node.output.emptyandorchestration.detail.node.logs.empty, butzh.mjsstill doesn't define them. In zh, empty sections will fall back to raw i18n keys.Suggested fix
'orchestration.detail.node.meta': '尝试 {attempts} 次 · 自动修复 {autoFix} 次', 'orchestration.detail.node.step': '任务步骤', + 'orchestration.detail.node.output.empty': '无输出', + 'orchestration.detail.node.logs.empty': '无日志', 'orchestration.openai.status.title': 'OpenAI Chat 运行配置',🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/i18n/locales/zh.mjs` around lines 1242 - 1259, Add the missing zh locale entries for the empty-state labels used by formatTaskNodeOutputText() and formatTaskNodeLogs(), since orchestration.detail.node.output.empty and orchestration.detail.node.logs.empty are currently undefined in zh.mjs. Update the locale object near the existing orchestration.detail.node.* strings so these keys have proper Chinese fallback text, matching the other task node labels.web-ui/partials/index/panel-orchestration.html (1)
269-270: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDrop
node.kindfrom the masked plan node header.Line 270 still appends
node.kind, so the plan preview is leaking raw step metadata even though this view was otherwise converted to neutral labels and counts.Suggested fix
- <div class="task-node-meta">{{ t('orchestration.detail.node.meta', { attempts: 0, autoFix: 0 }) }} · {{ node.kind }}</div> + <div class="task-node-meta">{{ t('orchestration.detail.node.meta', { attempts: 0, autoFix: 0 }) }}</div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/partials/index/panel-orchestration.html` around lines 269 - 270, Remove the raw node kind from the masked plan node header in the orchestration panel so the preview stays neutral. Update the header rendering in the plan node markup to rely only on the translated title and meta counts, and drop the `node.kind` append from the `task-node-meta` expression.web-ui/modules/i18n/locales/vi.mjs (1)
1145-1145: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFinish localizing the new visible labels.
These two additions are still English, so the Vietnamese workspace picker and Kilo host panel will show mixed-language copy.
Also applies to: 1212-1212
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-ui/modules/i18n/locales/vi.mjs` at line 1145, The new visible labels in the Vietnamese locale are still in English, so finish translating the workspace picker and Kilo host panel strings in the locale map. Update the entries for orchestration.workspace.choice.label and the other added label near the Kilo host panel section in vi.mjs so they read naturally in Vietnamese and match the surrounding translated keys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/unit/web-ui-logic.test.mjs`:
- Around line 1975-2022: The Kilo webview host snapshot test does not currently
cover redaction of conversationMessages, so raw prompt or assistant text could
still leak through createTaskKiloWebviewHostSnapshot. Update the snapshot helper
to redact or omit conversationMessages alongside the existing workspace, thread,
run, and ids sanitization, and extend this test to include a
conversationMessages payload and assert that the serialized snapshot does not
contain any raw message content.
In `@web-ui/modules/app.methods.task-orchestration.mjs`:
- Around line 900-936: `handleTaskKiloWebviewMessage` mutates
`this.taskOrchestration` directly when handling `sendMessage` and
`openSettingsPanel`, so make sure the orchestration state exists first. Call
`this.ensureTaskOrchestrationState()` at the start of
`handleTaskKiloWebviewMessage` (before reading or writing `chatDraft` or
`settingsOpen`) so the `taskOrchestration` object is always initialized before
any webview message can update it.
- Around line 938-952: Guard the `installTaskKiloWebviewShim` bootstrap against
`window.localStorage` access throwing in sandboxed or storage-disabled webviews,
and avoid replacing an existing `window.acquireVsCodeApi` bridge. Update the
`installTaskKiloWebviewShim` flow so it first checks whether a usable VS Code
API bridge already exists, then safely obtains storage (falling back if
unavailable) before calling `createTaskKiloVsCodeApiShim`, and only assigns
`window.acquireVsCodeApi` when no bridge is present.
In `@web-ui/modules/i18n/locales/ja.mjs`:
- Line 1065: The Kilo surface title string is still left in English, causing
mixed-language UI in the Japanese locale. Update the
`orchestration.kilo.surface.title` entry in `ja.mjs` to a proper Japanese
translation, keeping the key unchanged and matching the localization style used
by the other entries in this locale file.
In `@web-ui/modules/i18n/locales/zh-tw.mjs`:
- Line 1057: The orchestration.kilo.surface.title entry in the zh-TW locale is
still in English, so update this translation in zh-tw.mjs to a Traditional
Chinese string to keep the Kilo host UI consistent. Locate the locale mapping
for orchestration.kilo.surface.title and replace the current Webview surface
text with the proper zh-TW translation, matching the tone and terminology used
by nearby locale entries.
---
Outside diff comments:
In `@web-ui/modules/i18n/locales/vi.mjs`:
- Line 1145: The new visible labels in the Vietnamese locale are still in
English, so finish translating the workspace picker and Kilo host panel strings
in the locale map. Update the entries for orchestration.workspace.choice.label
and the other added label near the Kilo host panel section in vi.mjs so they
read naturally in Vietnamese and match the surrounding translated keys.
In `@web-ui/modules/i18n/locales/zh.mjs`:
- Around line 1242-1259: Add the missing zh locale entries for the empty-state
labels used by formatTaskNodeOutputText() and formatTaskNodeLogs(), since
orchestration.detail.node.output.empty and orchestration.detail.node.logs.empty
are currently undefined in zh.mjs. Update the locale object near the existing
orchestration.detail.node.* strings so these keys have proper Chinese fallback
text, matching the other task node labels.
In `@web-ui/partials/index/panel-orchestration.html`:
- Around line 269-270: Remove the raw node kind from the masked plan node header
in the orchestration panel so the preview stays neutral. Update the header
rendering in the plan node markup to rely only on the translated title and meta
counts, and drop the `node.kind` append from the `task-node-meta` expression.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5a95fcfe-6294-4633-ae9a-c55ecfb669b0
📒 Files selected for processing (15)
tests/unit/config-tabs-ui.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/modules/task-kilo-webview-host.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/task-orchestration.css
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/unit/config-tabs-ui.test.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/res/web-ui-render.precompiled.js
📜 Review details
🔇 Additional comments (4)
web-ui/modules/task-kilo-webview-host.mjs (1)
1-268: LGTM!web-ui/app.js (1)
610-612: LGTM!web-ui/modules/app.methods.task-orchestration.mjs (1)
273-296: LGTM!Also applies to: 346-373, 449-466, 502-553, 577-640, 749-858
web-ui/partials/index/panel-orchestration.html (1)
556-561: 🔒 Security & PrivacyVerify the masking fallback for these sensitive inputs.
These fields now depend on
task-sensitive-input, and the paired stylesheet change masks text with-webkit-text-security. If this panel ever runs outside a Chromium/WebKit webview,workspacePathandthreadIdwill render in clear text again.
|
Updated the orchestration tab again to address the UI direction: this is no longer a small compatibility card added to the old layout. What changed in this commit:
Validation performed after this rewrite:
Known existing noise:
@coderabbitai re-review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-ui/partials/index/panel-orchestration.html`:
- Line 65: The session picker is being artificially limited by the
taskOrchestrationWorkspaceSessions.slice(0, 10) usage in the panel-orchestration
template, which hides older sessions once there are more than ten. Update the
session rendering in this picker to use the full
taskOrchestrationWorkspaceSessions list, or introduce an explicit
show-more/loading pattern in the same UI so all sessions remain reachable from
the selector.
- Around line 219-225: Add a real masking fallback for the sensitive inputs in
the orchestration panel: `taskOrchestration.workspacePath` and
`taskOrchestration.threadId` currently depend only on `.task-sensitive-input` /
`-webkit-text-security`, so unsupported browsers show plain text. Update the
markup in `panel-orchestration.html` to use a password-style fallback or
equivalent masking behavior for these two fields, while preserving the existing
`v-model`, hints, and labels.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be6e9271-428a-47e1-bf5f-616119f68219
📒 Files selected for processing (4)
tests/unit/config-tabs-ui.test.mjsweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles/task-orchestration.css
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/config-tabs-ui.test.mjs
📜 Review details
🧰 Additional context used
🪛 Stylelint (17.14.0)
web-ui/styles/task-orchestration.css
[error] 3246-3246: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
|
Updated the PR after the full orchestration tab rewrite. What changed in the latest commit:
Validation performed:
Important scope note:
Known existing noise:
@coderabbitai re-review |
|
✅ Action performedFull review finished. |
|
Update on
Validation performed:
@coderabbitai re-review |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/e2e/test-task-orchestration.js (1)
182-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated config scaffolding between
writeOpenAiChatConfigandwriteOpenAiChatFallbackConfig.Both functions repeat the same
mkdirSynccalls, the identical[model_providers.local]TOML block, and near-identical JSON provider serialization. Any future change to the shared local-codex provider block or to the JSON provider shape now needs to be kept in sync across both functions (and any others repeating this pattern), risking silent drift betweenconfig.tomlandtask-openai-chat-provider.jsonfixtures.Consider extracting a shared helper (e.g.,
writeCodexBaseConfig(tmpHome, mode)andbuildOpenAiChatProviderJson(providers, selectedProvider)) that both functions call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/test-task-orchestration.js` around lines 182 - 281, The two fixture helpers duplicate the same setup for the shared Codex config and provider JSON, so extract the common pieces into reusable helpers. Move the repeated mkdirSync calls, the shared [model_providers.local] TOML section, and the common provider JSON construction in writeOpenAiChatConfig and writeOpenAiChatFallbackConfig into shared functions such as writeCodexBaseConfig(tmpHome, mode) and buildOpenAiChatProviderJson(...), then have both helpers call them and keep only the mode-specific/provider-specific parts inline.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web-ui/modules/i18n/locales/vi.mjs`:
- Around line 96-126: The new localization entries in vi.mjs are still raw
English, so update the affected i18n strings for the Local AI Service and
orchestration rebuilding UI with Vietnamese translations. Keep the same keys
under the locale object (including tab.localAi, side.localAi.meta,
kicker.localAi, title.localAi, subtitle.localAi, orchestration.rebuilding.*, all
localAi.* entries, and orchestration.openai.status.present) and translate them
consistently with the existing Vietnamese tone used elsewhere in vi.mjs.
In `@web-ui/partials/index/layout-header.html`:
- Around line 327-339: The Local AI tab meta in the layout header hardcodes the
word “providers” instead of using localization. Update the render logic in the
local-ai button section to replace the literal English suffix with an i18n
lookup, using a new locale key such as side.localAi.providerCount in the same
area that already calls t('tab.localAi') and t('side.localAi.meta'). Add the
corresponding translation entries to all locale files so the provider count
label is translated consistently.
In `@web-ui/partials/index/panel-orchestration.html`:
- Around line 25-27: The device settings gear in the panel is currently
hard-disabled and missing an action, so the modal cannot be opened from this
surface. Update the button in panel-orchestration.html to call
openTaskDeviceSettings() on click, and make disabled depend on the actual
rebuilding state instead of always being set; if this panel is not the right
entry point, move the launcher to the intended component.
---
Nitpick comments:
In `@tests/e2e/test-task-orchestration.js`:
- Around line 182-281: The two fixture helpers duplicate the same setup for the
shared Codex config and provider JSON, so extract the common pieces into
reusable helpers. Move the repeated mkdirSync calls, the shared
[model_providers.local] TOML section, and the common provider JSON construction
in writeOpenAiChatConfig and writeOpenAiChatFallbackConfig into shared functions
such as writeCodexBaseConfig(tmpHome, mode) and
buildOpenAiChatProviderJson(...), then have both helpers call them and keep only
the mode-specific/provider-specific parts inline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37da23a0-10c7-437b-b8cd-15fc12518e9e
📒 Files selected for processing (21)
cli.jstests/e2e/test-task-orchestration.jstests/unit/config-tabs-ui.test.mjstests/unit/web-ui-behavior-parity.test.mjstests/unit/web-ui-logic.test.mjsweb-ui/app.jsweb-ui/index.htmlweb-ui/modules/app.computed.main-tabs.mjsweb-ui/modules/app.methods.navigation.mjsweb-ui/modules/app.methods.task-orchestration.mjsweb-ui/modules/i18n/locales/en.mjsweb-ui/modules/i18n/locales/ja.mjsweb-ui/modules/i18n/locales/vi.mjsweb-ui/modules/i18n/locales/zh-tw.mjsweb-ui/modules/i18n/locales/zh.mjsweb-ui/partials/index/layout-header.htmlweb-ui/partials/index/panel-local-ai.htmlweb-ui/partials/index/panel-orchestration.htmlweb-ui/res/web-ui-render.precompiled.jsweb-ui/styles.cssweb-ui/styles/local-ai-service.css
✅ Files skipped from review due to trivial changes (2)
- web-ui/index.html
- web-ui/modules/i18n/locales/en.mjs
🚧 Files skipped from review as they are similar to previous changes (7)
- tests/unit/config-tabs-ui.test.mjs
- tests/unit/web-ui-behavior-parity.test.mjs
- tests/unit/web-ui-logic.test.mjs
- cli.js
- web-ui/modules/i18n/locales/zh-tw.mjs
- web-ui/modules/app.computed.main-tabs.mjs
- web-ui/modules/app.methods.task-orchestration.mjs
| 'field.endpoint': 'Endpoint', | ||
| 'field.type': 'Type', | ||
| 'tab.localAi': 'Local AI', | ||
| 'side.localAi.meta': 'Provider adapters', | ||
| 'kicker.localAi': 'Local service', | ||
| 'title.localAi': 'Local AI Service', | ||
| 'subtitle.localAi': 'Manage Claude/OpenAI providers for the local adapter service.', | ||
| 'orchestration.rebuilding.status': 'Rebuilding', | ||
| 'orchestration.rebuilding.title': 'Task tab is rebuilding', | ||
| 'orchestration.rebuilding.body': 'This tab is read-only while task orchestration is refactored. Configure providers in Local AI Service.', | ||
| 'orchestration.rebuilding.placeholder': 'Task input is disabled while this surface is rebuilding.', | ||
| 'localAi.action.open': 'Open Local AI Service', | ||
| 'localAi.action.newProvider': 'New provider', | ||
| 'localAi.kicker': 'Local adapter', | ||
| 'localAi.title': 'Local AI Service', | ||
| 'localAi.body': 'Add Claude or OpenAI-compatible providers. The local service stores provider metadata and exposes a provider-adapter layer for task execution.', | ||
| 'localAi.badge.localService': 'Local service', | ||
| 'localAi.badge.providerAdapter': 'Provider adapter', | ||
| 'localAi.badge.ready': 'Ready', | ||
| 'localAi.badge.needsProvider': 'Needs provider', | ||
| 'localAi.form.kicker': 'Provider setup', | ||
| 'localAi.form.title': 'Add Claude or OpenAI provider', | ||
| 'localAi.form.storeHint': 'Saved locally under ~/.codexmate; secrets are not rendered back.', | ||
| 'localAi.providers.kicker': 'Configured providers', | ||
| 'localAi.providers.title': 'Provider adapters', | ||
| 'localAi.provider.ready': 'Ready', | ||
| 'localAi.provider.incomplete': 'Incomplete', | ||
| 'localAi.provider.enabled': 'Enabled', | ||
| 'localAi.empty.title': 'No providers yet', | ||
| 'localAi.empty.body': 'Add an OpenAI or Claude provider to activate the local AI service.', | ||
| 'orchestration.openai.status.present': 'Present', |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
New keys left untranslated (English text in Vietnamese locale).
Unlike the equivalent additions in zh.mjs (properly translated to Chinese), these new keys — tab.localAi, side.localAi.meta, kicker.localAi, title.localAi, subtitle.localAi, orchestration.rebuilding.*, all localAi.* keys, and orchestration.openai.status.present — are left as raw English strings. Vietnamese users will see English copy across the entire new Local AI Service surface and the orchestration rebuilding banner.
Please provide Vietnamese translations for these values.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-ui/modules/i18n/locales/vi.mjs` around lines 96 - 126, The new
localization entries in vi.mjs are still raw English, so update the affected
i18n strings for the Local AI Service and orchestration rebuilding UI with
Vietnamese translations. Keep the same keys under the locale object (including
tab.localAi, side.localAi.meta, kicker.localAi, title.localAi, subtitle.localAi,
orchestration.rebuilding.*, all localAi.* entries, and
orchestration.openai.status.present) and translate them consistently with the
existing Vietnamese tone used elsewhere in vi.mjs.
| <button | ||
| id="side-tab-local-ai" | ||
| data-main-tab="local-ai" | ||
| :aria-current="mainTab === 'local-ai' ? 'page' : null" | ||
| :class="['side-item', { active: isMainTabNavActive('local-ai') }]" | ||
| @pointerdown="onMainTabPointerDown('local-ai', $event)" | ||
| @click="onMainTabClick('local-ai', $event)"> | ||
| <div class="side-item-title">{{ t('tab.localAi') }}</div> | ||
| <div class="side-item-meta"> | ||
| <span>{{ t('side.localAi.meta') }}</span> | ||
| <span>{{ taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }} providers</span> | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Hardcoded "providers" text bypasses i18n.
Line 337 renders {{ ... }} providers as a literal English string, while the sibling orchestration meta line above it uses t(...) for localization. This will not translate for vi/ja/zh locales.
🌐 Suggested fix using i18n
- <span>{{ taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }} providers</span>
+ <span>{{ t('side.localAi.providerCount', { count: taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }) }}</span>A new locale key (e.g. side.localAi.providerCount) would need to be added to all locale files.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| id="side-tab-local-ai" | |
| data-main-tab="local-ai" | |
| :aria-current="mainTab === 'local-ai' ? 'page' : null" | |
| :class="['side-item', { active: isMainTabNavActive('local-ai') }]" | |
| @pointerdown="onMainTabPointerDown('local-ai', $event)" | |
| @click="onMainTabClick('local-ai', $event)"> | |
| <div class="side-item-title">{{ t('tab.localAi') }}</div> | |
| <div class="side-item-meta"> | |
| <span>{{ t('side.localAi.meta') }}</span> | |
| <span>{{ taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }} providers</span> | |
| </div> | |
| </button> | |
| <button | |
| id="side-tab-local-ai" | |
| data-main-tab="local-ai" | |
| :aria-current="mainTab === 'local-ai' ? 'page' : null" | |
| :class="['side-item', { active: isMainTabNavActive('local-ai') }]" | |
| `@pointerdown`="onMainTabPointerDown('local-ai', $event)" | |
| `@click`="onMainTabClick('local-ai', $event)"> | |
| <div class="side-item-title">{{ t('tab.localAi') }}</div> | |
| <div class="side-item-meta"> | |
| <span>{{ t('side.localAi.meta') }}</span> | |
| <span>{{ t('side.localAi.providerCount', { count: taskOrchestration.localAiServiceStatus && taskOrchestration.localAiServiceStatus.providerOptions ? taskOrchestration.localAiServiceStatus.providerOptions.length : 0 }) }}</span> | |
| </div> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-ui/partials/index/layout-header.html` around lines 327 - 339, The Local
AI tab meta in the layout header hardcodes the word “providers” instead of using
localization. Update the render logic in the local-ai button section to replace
the literal English suffix with an i18n lookup, using a new locale key such as
side.localAi.providerCount in the same area that already calls t('tab.localAi')
and t('side.localAi.meta'). Add the corresponding translation entries to all
locale files so the provider count label is translated consistently.
| <button type="button" class="kilo-icon-button kilo-settings-button" disabled :aria-label="t('orchestration.rebuilding.status')" :title="t('orchestration.rebuilding.status')"> | ||
| ⚙ | ||
| </button> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n --no-heading 'kilo-settings-button|openTaskDeviceSettings|device.*[Mm]odal|kilo\.device\.open' web-ui/modules web-ui/partials -C 3Repository: SakuraByteCore/codexmate
Length of output: 5876
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- app.methods.task-orchestration.mjs around openTaskDeviceSettings ---'
sed -n '400,520p' web-ui/modules/app.methods.task-orchestration.mjs
echo
echo '--- search settingsOpen and device modal wiring ---'
rg -n --no-heading 'settingsOpen|openTaskDeviceSettings|orchestration\.kilo\.device\.open|orchestration\.kilo\.device\.title|orchestration\.kilo\.device\.body' web-ui -C 2
echo
echo '--- panel-orchestration.html around the settings button and surrounding state ---'
sed -n '1,180p' web-ui/partials/index/panel-orchestration.htmlRepository: SakuraByteCore/codexmate
Length of output: 24196
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- all settingsOpen references in web-ui ---'
rg -n --no-heading 'settingsOpen' web-ui -C 2
echo
echo '--- all openTaskDeviceSettings references in web-ui ---'
rg -n --no-heading 'openTaskDeviceSettings' web-ui -C 2
echo
echo '--- task orchestration template sections that mention device/settings modal terms ---'
rg -n --no-heading 'orchestration\.kilo\.device\.open|orchestration\.kilo\.device\.title|orchestration\.kilo\.device\.body|task-device|device-settings|settings-open|settingsOpen' web-ui/partials web-ui/modules -C 3Repository: SakuraByteCore/codexmate
Length of output: 10104
Device settings launcher is disabled here
This gear is hard-disabled and has no @click, so the new device modal has no entry point from this panel. Wire it to openTaskDeviceSettings() and gate disabled on the actual rebuilding state, or move the launcher to the intended surface.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-ui/partials/index/panel-orchestration.html` around lines 25 - 27, The
device settings gear in the panel is currently hard-disabled and missing an
action, so the modal cannot be opened from this surface. Update the button in
panel-orchestration.html to call openTaskDeviceSettings() on click, and make
disabled depend on the actual rebuilding state instead of always being set; if
this panel is not the right entry point, move the launcher to the intended
component.
a4320f9 to
5a9642b
Compare
533b143 to
37d5524
Compare
37d5524 to
3cfc27f
Compare
Summary
codexmate kilo/codexmate kilocodelaunch KiloCode instead of being config-only commandscodexmate kilo config <url> <key> <model> [--provider <id>]for pure KiloCode provider setupcodexmate/gpt-5.3@kilocode/cli,kilo/kilocode) to CLI install status and Docs install commands<picture>variant failed to display reliably on GitHub0.1.3and publish git tagv0.1.3on the latest PR headinstall-statusValidation
node --check cli.jsnode --check web-ui/app.jsnode --test tests/unit/kilocode-config-cli.test.mjsnpm run test:unitnpm run test:e2enpm run lintnpm testgrepNotes